Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename ElasticSearch* to Elasticsearch* #4634

Closed
s1monw opened this issue Jan 6, 2014 · 7 comments
Closed

Rename ElasticSearch* to Elasticsearch* #4634

s1monw opened this issue Jan 6, 2014 · 7 comments

Comments

@s1monw
Copy link
Contributor

s1monw commented Jan 6, 2014

before we release 1.0 we should cleanup naming on the internal classes to use Elasticsearch rather than ElasticSearch

@ghost ghost assigned s1monw Jan 6, 2014
@s1monw s1monw closed this as completed in fa16969 Jan 7, 2014
@s1monw
Copy link
Contributor Author

s1monw commented Jan 8, 2014

For the record this issue renamed the following classes:

  • ElasticSearchException.javaElasticsearchException.java
  • ElasticSearchGenerationException.javaElasticsearchGenerationException.java
  • ElasticSearchIllegalArgumentException.javaElasticsearchIllegalArgumentException.java
  • ElasticSearchIllegalStateException.javaElasticsearchIllegalStateException.java
  • ElasticSearchInterruptedException.javaElasticsearchInterruptedException.java
  • ElasticSearchNullPointerException.javaElasticsearchNullPointerException.java
  • ElasticSearchParseException.javaElasticsearchParseException.java
  • ElasticSearchTimeoutException.javaElasticsearchTimeoutException.java
  • ElasticSearchWrapperException.javaElasticsearchWrapperException.java
  • ElasticSearch.javaElasticsearch.java
  • ElasticSearchF.javaElasticsearchF.java

This change is not backwards compatible!

@dadoonet
Copy link
Member

dadoonet commented Jan 9, 2014

With that change, all plugins are broken for 1.0.
Trying to keep plugins compatible with both 0.90 and 1.0 like we started to do in https://github.com/dadoonet/elasticsearch-cloud-aws/commit/72595560be16d78c335a110cb659c455a15fda33#diff-29637d427c53f19f7a083fd0a40da1b5R89 is not doable anymore.

That basically means that all plugins in their master branch are not compatible anymore with 0.90. I will update all README.

See also https://github.com/elasticsearch/elasticsearch-transport-memcached/issues/13
and https://github.com/elasticsearch/elasticsearch-transport-thrift/issues/12

@s1monw
Copy link
Contributor Author

s1monw commented Jan 9, 2014

@dadoonet yes that is correct we need to see how we deal with major version updates in the plugins. What keeps us from doing version branches there as well aligned with the ES versions?

@dadoonet
Copy link
Member

dadoonet commented Jan 9, 2014

I did propose that but @kimchy would like to keep it simple.
I agree that we should create branch for plugins only when we need to fix an issue.

So I guess we don't need to do it now.

brusic pushed a commit to brusic/elasticsearch that referenced this issue Jan 19, 2014
 * Clean up s/ElasticSearch/Elasticsearch on docs/*
 * Clean up s/ElasticSearch/Elasticsearch on src/* bin/* & pom.xml
 * Clean up s/ElasticSearch/Elasticsearch on NOTICE.txt and README.textile

Closes elastic#4634
@apatrida
Copy link
Contributor

Wow, ok, so all plugins that were already falling behind trying to keep up with previous broken changes are now one more step back. For a cosmetic change, was it worth breaking most plugins and most client code? (not just this change, but HPCC non Map implementing Map-like classes and others)

This would be nice change to also include pull requests to many plugins that are now unload-able.

@s1monw
Copy link
Contributor Author

s1monw commented Jan 20, 2014

@jaysonminard the plugins that we have access to are already fixed. If you have a plugin you will be able to fix that in a couple of minutes. The chance to fix stuff like this for consistency doesn't happen too often and I think it had enough time to bubble up before RC1 now that we have a RC1 you can easily upgrade and make your plugin loadable for the GA release. Which plugin doesn't load for you?

@apatrida
Copy link
Contributor

@s1monw there are plugins like IN/OUT plugin which are not even accepting pull requests (they didn't have time to keep up with breaking changes) that we have had to fork, and we'll bring it up to date along with our other forks. But I'm talking generally and not just in this particular change. Over the last 2 months it is more common to find plugins that are not currently load-able or run-able (even if loading) or sometimes just in an unknown and untested state with current ES versions. Obviously breaking changes have to happen sometimes along the way, but the rate seems to increase towards 1.0 rather than reduce towards stability. So while ES might be more stable, the ecosystem around it seems more in disarray.

And even if a plugin works, the documentation doesn't make it clear if they will still load since they are not updated to reflect the top version supported. Imagine a new ES user installing it, trying to use a plugin listed on the site but not seeing a matching version, and sometimes using one to find it doesn't work. Does not lend credibility to the product. I think this is my message, please keep that in mind (you likely are, but it feels less so from the "outside")

I think it is a case where the speed of change has outpaced the open-source plugins writers ability or desire to keep up with the pace of change.

(The previous collection class changes just introduced really odd feeling code to users of the Java client, that's a different story. I think the example of listing indexes with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants